Skip to content

Reject duplicate partition field names - #4305

Open
sohailshk wants to merge 2 commits into
apache:mainfrom
sohailshk:fix/partition-name-unique
Open

sohailshk wants to merge 2 commits into
apache:mainfrom
sohailshk:fix/partition-name-unique

Conversation

@sohailshk

Copy link
Copy Markdown

Closes #4304

Rationale for this change

validate_partition_name returned early when the partition name was not also a schema column, so its empty-name and uniqueness checks only ran for names that matched a column. update_spec could then commit a spec with two fields of the same name (for example through rename_field), and create_table did not check uniqueness at all because assign_fresh_partition_spec_ids passed a fresh set of names for every field.

This runs the empty-name and uniqueness checks first, and tracks the names seen so far in assign_fresh_partition_spec_ids.

Are these changes tested?

Yes:

  • test_update_spec_rejects_duplicate_partition_name in tests/table/test_init.py: renaming a field to an existing name, adding a field with a name that is being renamed away, and an empty name now raise.
  • test_new_table_metadata_with_duplicate_partition_name_fails in tests/table/test_metadata.py: new table metadata with two partition fields of the same name now raises.

Both fail on main and pass with this change. tests/table and tests/test_schema.py: 890 passed. ruff check and ruff format --check (0.14.3) pass on the changed files.

Are there any user-facing changes?

Partition specs with duplicate field names, which were accepted before, now raise ValueError: Partition name has to be unique: <name>.

validate_partition_name returned early when the partition name was not
also a schema column, so its empty-name and uniqueness checks only ran
for names that matched a column. update_spec could then commit a spec
with two fields of the same name (for example through rename_field),
after which appends failed and Java cannot load the metadata.
create_table did not check uniqueness at all because it passed a fresh
set for every field.

Run the empty-name and uniqueness checks first, and track the names seen
so far in assign_fresh_partition_spec_ids.
Comment thread tests/table/test_init.py Resolved

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update_spec and create_table accept two partition fields with the same name

2 participants